Skip to content

fix(internet)!: keep MH option 6 prefixes with host bits and option 27 times past 9999 (#2702) - #2709

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2702-mh-valueerror
Nov 3, 2026
Merged

fix(internet)!: keep MH option 6 prefixes with host bits and option 27 times past 9999 (#2702)#2709
JarryShaw merged 1 commit into
mainfrom
fix/2702-mh-valueerror

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Nov 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

Two MH mobility options raised a bare ValueError when parsed on their own. Neither RFC has the receiver discard these values, so both now keep what they parse:

  • Option 6 (MNP, RFC 3963): prefix is the captured IPv6Address, with a new prefix_length field, the same shape as its sibling Home Network Prefix. A prefix with host bits set, and a prefix length past 128, now parse and rebuild. _make_opt_mnp checks the prefix length against its 8-bit field only, and still takes the network forms.
  • Option 27 (Timestamp, RFC 5213): timestamp is None past the year 9999, and pmip_timestamp keeps the raw value. The reader no longer adds seconds and fraction as one float, which raised at the last second of 9999 and drifted by up to about 15 µs above 2^36 seconds.

Breaking: the MNP data model changes: prefix is an address, and prefix_length is a new required field. The Timestamp's timestamp becomes Optional. _make_opt_mnp also takes an int prefix.

The new tests cover standalone parse, parsing inside IPv6 (quiet), from_data from info and from to_dict(), and make(), at the edge values. They fail on main.

Closes #2702

@JarryShaw JarryShaw added this to the PyPCAPKit v1.6 milestone Nov 3, 2026
@JarryShaw JarryShaw added bug Issues reporting a defect (set by the bug report template; a default, not an assessment) breaking Breaks public-facing behaviour or API (apply alongside the type label) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 3, 2026
@JarryShaw JarryShaw added the review: running A cross-review is in flight against the current head - no verdict yet label Nov 3, 2026
@JarryShaw JarryShaw mentioned this pull request Nov 3, 2026
Closed
@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 3, 2026
@JarryShaw

JarryShaw commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

GOOD TO GO at c8e3e0eb7, from a Sonnet cross-review.

  • RFCs: RFC 3963 §6.2 and RFC 5213 §5.5 answer a bad value with a status, not a format discard, so keeping the values is right.
  • MNP: 3000 random cases round-trip exactly.
  • Timestamp: it matches an exact-fraction oracle on 2.4M cases, where the old float sum missed 360k.
  • Captures: they are identical apart from the intended MNP model change.
  • The break is justified: an IPv6Network cannot hold host bits or a length past 128.

One small follow-up goes into this PR: Home Network Prefix (option 22) still refuses a prefix length past 128, where MNP now keeps it. It gets the same wire-width rule. The body is updated for the Optional timestamp and the int prefix.

@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 3, 2026
@JarryShaw
JarryShaw force-pushed the fix/2702-mh-valueerror branch from c8e3e0e to adec521 Compare November 3, 2026 01:11
@JarryShaw

JarryShaw commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Pushed c8e3e0eb7→adec5211a, which folds in the HNP follow-up, and I checked the delta myself. Home Network Prefix (option 22) now checks its prefix length only against the 8-bit field, the same as MNP. RFC 5213 §5.3.2 answers an unauthorised prefix with status 155, not a discard. Its data model already keeps the address and length apart, so this adds no new break. The internet leg passes. This stays GOOD TO GO.

@JarryShaw

JarryShaw commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at adec5211a: Python 3.12 and 3.14 are red, and the failure reproduces locally.

TestMHOnWindows.test_timestamp_option_after_3000 in tests/protocols/misc/test_fromtimestamp_windows_range_2099_unit.py still expects a ValueError for an option 27 time past year 9999. This PR now keeps such a time, so update the test's final block to assert the new behaviour. Then sweep the rest of tests/ for anything else that pins the old one.

The review missed it because it only ran the internet tests, and this test lives in the misc tests.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Nov 3, 2026
@JarryShaw
fix(internet)!: keep MH option 6 prefixes with host bits and option 2…
0dc9a95 
…7 times past 9999 (#2702)

- The Mobile Network Prefix option fused its prefix and length into an
  IPv6Network, so a prefix with bits set past its length, or a length past
  128, raised a bare ValueError. RFC 3963 section 6.2 reads both fields as
  given and answers a prefix it cannot use with a status, not a discard, so
  the data model keeps them apart, as Home Network Prefix does: prefix_length
  is a new required field and prefix is the IPv6Address captured, the breaking
  change. The maker checks the length against its 8-bit field alone, takes a
  prefix_length keyword, and writes the bits past it.
- The Home Network Prefix option refused a length past 128 to read and to
  make. RFC 5213 section 5.3.2 answers a prefix it cannot authorise with a
  status, so both take the 8-bit field's width, as option 6 does.
- The Timestamp option turned a 48-bit second count past 9999 into a datetime,
  which raised a bare ValueError. RFC 5213 section 5.5 has the LMA answer such
  a time with a status rather than discard it, so timestamp is None there and
  pmip_timestamp keeps it. The seconds and fraction are no longer summed as one
  float, which carried 23:59:59 and 65535/65536 past the year's end.
- The mutation sweep's known crashes and the maker sweep's unparsed maximum go,
  and the Windows-range test reads a time past 9999 as Linux does, not raising.

New tests fail on 57a6c36 without the pcapkit/ change; MH and guard modules pass.
@JarryShaw

JarryShaw commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Fold pushed: adec5211a→0dc9a95ad, still one commit on 57a6c366d. It merges cleanly with main.

  • Only the stale pin changed, in TestMHOnWindows.test_timestamp_option_after_3000. It now checks that a time past 9999 reads the same under windows_gmtime() as on Linux, keeps pmip_timestamp, and rebuilds the original octets. There is no diff under pcapkit/.
  • A sweep of all of tests/ found no other pin of the old behaviour. The misc, internet and project test directories pass, one module per process.

Since only a test changed after the reviewed head, the verdict is GOOD TO GO.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Nov 3, 2026
@JarryShaw
JarryShaw force-pushed the fix/2702-mh-valueerror branch from adec521 to 0dc9a95 Compare November 3, 2026 02:24
@JarryShaw
JarryShaw merged commit 37ee3cc into main Nov 3, 2026
54 checks passed
@JarryShaw
JarryShaw deleted the fix/2702-mh-valueerror branch November 3, 2026 02:28
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 3, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 3, 2026
JarryShaw added a commit that referenced this pull request Nov 3, 2026
@JarryShaw
docs(changelog): fold make()'s enumeration fields, the corekit.schema…
d2aee5b 
… helpers, MH options 6 and 27, the cut SCTP and option-area schemas and dict-built secrets into 1.6.0 (#2449)

- Breaking: a make() enumeration field takes a member or an int, its
  X_default/X_namespace/X_reversed keywords removed with no alias, and names,
  wide codes and wide option codes are refused (#2692); the schema helpers
  move to pcapkit.corekit.schema and pcapkit.corekit.fields.byteorder is
  removed (#2704); MobileNetworkPrefixOption gains prefix_length and keeps an
  IPv6Address prefix, and TimestampOption's timestamp is None past 9999
  (#2709). Folded into the MH data-model entry; old values measured against
  v1.5.1.
- Fixed: the SCTP schema cut inside its padding and a header schema cut
  inside an option area pack back as captured (#2682), into the SCTP entry;
  a Decryption Secrets Block rebuilds from its dict as captured (#2707), into
  the DSB entry.
- Folded into unreleased entries: FRAD's Q.922 address refusal and quiet cut
  rebuild (#2719); a from_data rebuild's checksum status (#2712); a failed
  ListField fill's ledger (#2706); pad_length's path (#2704); TLS's enumeration
  fields (#2692); the make() and schema-helper rulings, into the conventions
  entry (#2692, #2704); the GraalPy shards, into the CI-legs entry (#2720).
- 1.5.0's corekit.fields.byteorder role points at corekit.schema.byteorder,
  where #2704 moved it, so the nitpicky build does not warn on it; 1.5.0's
  text unchanged.
- No entry for the test-only 72b2bd8 (#2715), or the docs-and-test-only
  0c07940 (#2718).
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
This was referenced Nov 3, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking Breaks public-facing behaviour or API (apply alongside the type label) bug Issues reporting a defect (set by the bug report template; a default, not an assessment)

Projects

PyPCAPKit
Status: Done

Milestone

PyPCAPKit v1.6

Development

Successfully merging this pull request may close these issues.

fix(internet): MH options 6 and 27 raise a bare ValueError on values their data models cannot hold

1 participant

@JarryShaw